Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary polyfill.io #3763

Merged
merged 1 commit into from
Mar 2, 2024

Conversation

SukkaW
Copy link
Contributor

@SukkaW SukkaW commented Mar 1, 2024

Pull Request Checklist

  • I’ve read the guidelines for contributing.
  • I updated AUTHORS.txt and CHANGES.txt (if the change is non-trivial) and documentation (if applicable).
  • I tested my changes.

Description

I haven't updated the AUTHORS.txt as I though small changes like this should not qualify.

The PR replaces all polyfill.io with cdnjs.cloudflare.com/polyfill in the docs and the template.

polyfill.io was acquired by a China-based CDN company "Funnull", see the announcement from the polyfill.io domain owner's Twitter and https://github.com/polyfillpolyfill/polyfill-service/issues/2834. Despite Funnull's claims of operating in the United States, the predominance of Simplified Chinese on its website suggests otherwise, and it turns out that "Funnull" is notorious for providing service for the betting and pornography industries.

The original creator of the polyfill.io has voiced his concern on Twitter. And since the acquisition, numerous issues have emerged (polyfillpolyfill/polyfill-service#2835, polyfillpolyfill/polyfill-service#2838, alist-org/alist#6100), rendering the polyfill.io service extremely unstable. Since then, Fastly (Announcement) and Cloudflare (Announcement) has hosted their own instances of polyfill.io service.

Copy link
Contributor

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution!

CHANGES.txt Outdated Show resolved Hide resolved
@Kwpolska
Copy link
Member

Kwpolska commented Mar 1, 2024

I’m wondering if we actually need the polyfills. They were required for Safari, but Safari seems to have added this feature in 2020. Do you have access to a Mac to verify?

@SukkaW
Copy link
Contributor Author

SukkaW commented Mar 2, 2024

I’m wondering if we actually need the polyfills. They were required for Safari, but Safari seems to have added this feature in 2020. Do you have access to a Mac to verify?

I only have a Apple-Silicon powered Mac thus can't install the Safari 14. But many repositories I've opened a PR to are suggesting removing the polyfill entirely, and we could do the same.

On the other hand, removing the polyfill could be a breaking change though.

@Kwpolska
Copy link
Member

Kwpolska commented Mar 2, 2024

Could you still test in whichever newer Safari version you have? If the feature actually works there in different languages, we can remove the polyfill altogether (we’ll just assume the information about 2020/Safari 14 is correct), Apple users will probably be on a recent version anyway.

@SukkaW
Copy link
Contributor Author

SukkaW commented Mar 2, 2024

Could you still test in whichever newer Safari version you have? If the feature actually works there in different languages, we can remove the polyfill altogether (we’ll just assume the information about 2020/Safari 14 is correct), Apple users will probably be on a recent version anyway.

So I opened the MDN page on my Safari (Safari 17.4) and ran the example, it does work though.

image

@Kwpolska
Copy link
Member

Kwpolska commented Mar 2, 2024

Great, in that case, please remove the polyfill altogether :)

@aknrdureegaesr
Copy link
Contributor

Side remark: For web sites that try to comply with EU GDPR, one needs a signed written agreement of a certain kind with any CDN or similar entity, before sending traffic their way. I vividly remember a concerted effort across many dev teams to remove Google fonts from all web presences of a big German business that was my employer at the time. From a legal point of view, it is much safer to self-hosting all resources needed.

@felixfontein felixfontein mentioned this pull request Mar 2, 2024
3 tasks
@SukkaW
Copy link
Contributor Author

SukkaW commented Mar 2, 2024

@felixfontein @Kwpolska I've rebased the PR and all polyfill.io usages are removed.

@Kwpolska Kwpolska changed the title Replace polyfill.io Remove unnecessary polyfill.io Mar 2, 2024
@Kwpolska Kwpolska enabled auto-merge (squash) March 2, 2024 12:57
@Kwpolska Kwpolska merged commit f7c2e91 into getnikola:master Mar 2, 2024
11 checks passed
@Kwpolska
Copy link
Member

Kwpolska commented Mar 2, 2024

Thanks for bringing this to our attention and fixing it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants